Conversation
🦋 Changeset detectedLatest commit: 9fc1125 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 363f39a39a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| yield* waitForAskThread(application, output, { | ||
| threadId, | ||
| format: resolvedFormat, | ||
| }); |
There was a problem hiding this comment.
Wait for the dispatched turn to become visible before watching
When dispatch acknowledgement arrives before the shell/thread projection has applied that sequence, this subscription can receive the pre-dispatch idle snapshot. watchThread immediately emits done for such an inactive, previously completed thread (and for a newly created thread whose turn is not yet visible), so ask proceeds to the final snapshot, reports AskNoAnswerError, and may leave the requested turn running. The existing startThread/sendThread completion paths avoid this race by awaiting dispatch.sequence before opening the watcher; this path needs the same visibility barrier.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c757e237b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const threadId = state.threadId; | ||
| return Effect.gen(function* () { | ||
| let shouldInterrupt = state.created || state.baselineActiveTurnId === null; |
There was a problem hiding this comment.
Verify turn ownership before interrupting the active turn
When an existing-thread ask starts from an idle thread, baselineActiveTurnId is null, so cancellation always sets shouldInterrupt even if the ask's turn has already finished and another client has since started a different turn. interruptThread then targets the thread's current active turn, allowing a timeout or Ctrl-C during that race to terminate unrelated follow-up work; record the dispatched turn identity and confirm it is still active before interrupting.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5ceaa0354
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const userIndex = newMessages.findLastIndex((message) => message.role === "user"); | ||
| if (userIndex === -1) { | ||
| return undefined; | ||
| } | ||
| const turnId = thread.latestTurn?.turnId; |
There was a problem hiding this comment.
Bind answer selection to the dispatched ask turn
If another client sends a follow-up after this ask's watcher observes completion but before the final snapshot is loaded, newMessages.findLastIndex(...) selects that unrelated client's user message and latestTurn selects its turn. The command can consequently return the other turn's answer if it finishes quickly, or report AskNoAnswerError while it is still running, despite this ask having completed successfully. Select the response using the dispatched ask's turn/message identity rather than the last new user and latest thread turn.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fabd0c7ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
|
|
||
| const finalSnapshot = yield* application.getThreadMessages({ threadId }); | ||
| if (finalSnapshot.thread.session?.status === "error") { |
There was a problem hiding this comment.
Scope session errors to the dispatched ask turn
When another client starts a follow-up after this ask's watcher observes completion but before getThreadMessages returns, an error in that unrelated follow-up makes this thread-global status check fail an ask that already produced a valid answer. The fresh evidence after the answer-selection fix is that the error check still runs before the response is selected by askMessageId/askTurnId; only treat an error as fatal when it belongs to the dispatched ask turn, or prefer its already-completed bound answer.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36762cda7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const visibleActiveTurnId = visibleSummary.session?.activeTurnId ?? null; | ||
| if ( | ||
| visibleActiveTurnId !== null && | ||
| (state.created || visibleActiveTurnId !== state.baselineActiveTurnId) | ||
| ) { | ||
| state.askTurnId = visibleActiveTurnId; |
There was a problem hiding this comment.
Avoid treating the thread's next active turn as the ask turn
When this ask finishes without an assistant message and another client starts a turn before getThreadSummary completes, visibleActiveTurnId belongs to that other client but is stored as askTurnId; a timeout then passes it to interruptThreadTurn and terminates the unrelated turn. Fresh evidence in the current ownership fix is that the ID is still seeded from the thread-global active turn without binding it to askMessageId. Only record a turn ID after associating it with the dispatched message.
Useful? React with 👍 / 👎.
| } | ||
| }), | ||
| ), | ||
| Stream.runLast, |
There was a problem hiding this comment.
Stop watching when the dispatched ask completes
When another client starts a follow-up before this subscription receives its initial snapshot, that snapshot can already contain this ask's finalized answer while the thread is active for the other turn. watchThread emits done only when the whole thread becomes inactive, and Stream.runLast has no completion condition tied to messageId or askTurnId, so ask waits for unrelated work or times out despite its answer already being available. End the wait based on the dispatched ask rather than thread-global idleness.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6c64e4346
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| action: "ask", | ||
| }); | ||
|
|
||
| if (isThreadActive(summary)) { |
There was a problem hiding this comment.
Base busy decisions on a full thread snapshot
When an idle thread has a ready session but a stale latestTurn.state === "running", this shell summary is classified as active because it has no messages; the repository explicitly covers that valid completed state in src/domain/thread-lifecycle.test.ts. Consequently the default --busy fail rejects a completed thread, while --busy queue repeatedly finishes waitForBusyThread, reloads the same shell state, and never dispatches. Use the full thread completion state returned by the watcher (or otherwise account for terminal sessions) for this busy decision.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95172e25a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| }), | ||
| ), | ||
| Stream.takeUntil((event) => answerFound || event.type === "done"), |
There was a problem hiding this comment.
Wait for the ask turn to finish before accepting an answer
When a tool-using turn emits a completed, non-streaming assistant message and then continues running tools or producing later assistant messages, answerFound becomes true and this condition closes the watcher immediately. The final snapshot can therefore return that intermediate text, and a newly created ask may archive the thread while its turn is still active. Completion needs to be tied to the dispatched turn reaching a terminal state, not merely to one assistant message finishing.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45ad874824
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| answerFound && | ||
| input.state.askTurnId !== null && | ||
| thread.session?.activeTurnId !== input.state.askTurnId | ||
| ) { | ||
| turnComplete = true; |
There was a problem hiding this comment.
Stop waiting when an answerless ask turn becomes idle
When the dispatched turn reaches a normal idle state without emitting any assistant message, answerFound remains false, so this condition never marks the turn complete. The underlying watchThread also omits its done event in this case because isThreadCompleteEnough requires an assistant message for a normal terminal session. With the default unlimited timeout, ask therefore waits forever instead of reaching the existing AskNoAnswerError path.
Useful? React with 👍 / 👎.
| if (event.type === "status") { | ||
| const thread = yield* ensureNoPendingRequest(application, input.threadId); |
There was a problem hiding this comment.
Scope pending-request failures to the dispatched ask
When this ask has completed and another client starts a follow-up that requests approval or user input before the watcher processes its status, this thread-global check fails the already-successful ask. In an initial snapshot, the preceding thread event can already identify the bound answer, but this status handler still calls ensureNoPendingRequest before marking that ask turn complete; pending state should be associated with the dispatched ask rather than any later turn.
Useful? React with 👍 / 👎.
| value: Option.getOrUndefined(worktree), | ||
| scope: t3CliEnv.scope, | ||
| }); | ||
| const result = yield* application.startThread( |
There was a problem hiding this comment.
Preserve created-thread state before awaiting turn dispatch
For a newly created ask, startThread dispatches thread.create and then waits for its shell sequence before dispatching the turn, but state.threadId is assigned only after the entire call returns. If --timeout expires or the process is interrupted during that visibility wait, cleanup sees no dispatched thread and skips --archive always or --archive on-failure, leaving the newly created empty thread behind despite the requested policy.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e57495ac0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (answer === undefined) { | ||
| if (finalSnapshot.thread.session?.status === "error") { |
There was a problem hiding this comment.
Reject failed ask turns before returning partial answers
When the watcher opens after this ask has already entered an error state, its initial thread event can set turnComplete and stop the stream before the following status event performs the scoped error check. If the failed turn emitted any completed assistant message before failing, selectAskAnswer returns that partial text, and this conditional skips the session-error check entirely, causing ask to exit successfully with an incomplete answer. Check whether the bound ask turn failed before accepting its selected response.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c1aa2f54c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ? application.showThread(threadId).pipe(Effect.flatMap(ensureNoPendingRequest)) | ||
| : Effect.void, | ||
| ), | ||
| Stream.runLast, |
There was a problem hiding this comment.
Stop queue waits when an answerless busy turn becomes idle
When --busy queue watches an active turn that reaches a normal terminal session without producing an assistant message, watchThread never emits done because isThreadCompleteEnough requires an assistant message, so Stream.runLast waits forever when no timeout was supplied and the ask is never dispatched. The prior answerless-turn fix only covers the newly dispatched ask in inspectAskTurn; this separate busy-turn wait still needs to finish based on the full thread's active state rather than the watcher’s done event.
Useful? React with 👍 / 👎.
| const turnId = | ||
| knownTurnId ?? messageTurnId ?? requestedTurnId ?? thread.session?.activeTurnId ?? null; |
There was a problem hiding this comment.
Prefer the requested turn when steering a busy thread
With --busy steer, the previously active turn can emit an assistant message after this ask's user message has been appended. In that snapshot messageTurnId belongs to the old turn while requestedTurnId exactly identifies the turn requested by this ask, but this precedence selects the old ID; subsequent completion and answer selection can therefore return the previous turn's output or interrupt the wrong turn on timeout. Prefer the timestamp-bound requested turn over assistant messages that may belong to the pre-existing active turn.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fc1125ad8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sessionRunning = | ||
| thread.session?.status === "starting" || thread.session?.status === "running"; | ||
| const complete = | ||
| nextUserIndex !== -1 || |
There was a problem hiding this comment.
Keep waiting when the active ask turn is steered
When another client sends a user message while this ask's turn is still running, the supported busy-thread send path steers that same turn, but nextUserIndex !== -1 marks the ask complete immediately. The watcher then stops before the shared turn reaches a terminal state, and selectAskAnswer also excludes assistant output after that user message, so ask can return an intermediate answer or fail with AskNoAnswerError while its turn continues running. Treat a later user message as a boundary only when it belongs to a subsequent turn.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this out intentionally. The ask command rejects threads that are already busy. Concurrent mutation by another client after dispatch is outside the command contract. Supporting it would require shared-turn coordination and would add lifecycle complexity for a race that is not part of the intended one-shot workflow.
Summary
t3cli askfor one-shot project questions and explicit thread follow-upsValidation
pnpm format:checkpnpm lintpnpm typecheckpnpm build